Repository navigation
Review round 2: fail loudly on bad inputs, report partial results - #51
Merged
Merged
Conversation
- Multi-XYZ reader raises on a block that doesn't match its atom count instead of skipping it or swallowing the next one. - Charge and multiplicity are checked against the electron count (SMILES, XYZ, dry run); a radical as a singlet is an error. - gpuma's own XYZ comments (Charge/Multiplicity) are read back. - prune_symmetry falls back to Kabsch RMSD for a molecule RDKit can't kekulize; GetBestRMS gets gpuma's automorphism cap as maxMatches. - conformer_seed and the configured multiplicity reach convert, generate, optimize --smiles and smiles. - A failed batch retries its structures one at a time; batch results keep each input's metadata. - New optimization.max_steps (both paths); unconverged structures are marked (converged, "Not converged") and give exit 4, as do failed ensemble conformers. Non-finite energies count as failures. - The ASE path honours the energy convergence criterion. - --xyz-dir raises on an unreadable file and names the source files. - convert and generate check --device. - Config: batch_optimization_mode, logging_level, integer charge and multiplicity, steps_between_swaps >= 1 are validated; model_cache_dir is Fairchem-only and must be creatable. - No empty output file when every structure fails; cached models select their GPU again; ORB's TF32 is scoped to ORB runs; the summary names a loaded checkpoint (model.path).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Second review round. Each item fails loudly or reports a partial result where gpuma used to carry on silently.
Inputs
--dry-run. For example,[CH3]as a singlet is now an error.| Charge: … | Multiplicity: …) are read back and take precedence over the configured values, with a warning when they differ.--xyz-dir: an unreadable file is an error naming the file, and output comments name the source file of each structure.convertandgeneratecheck--device. A GPU that doesn't exist no longer falls back to the CPU silently.conformer_seedand the configured multiplicity now reachconvert,generate,optimize --smilesandsmiles.Optimization
metadata.optimization.max_steps(default 10000) applies to both paths; the ASE path had no limit before.converged: falseandNot convergedin the XYZ comment, and the CLI exits with code 4.energy_convergence_criterion. It uses torch-sim's test, the energy change of one step, instead of switching to the force default.ensemblereportspartial(exit 4) when conformers fail, and the summary addsn_generated.Conformer pruning
prune_symmetryfalls back to Kabsch RMSD for a molecule RDKit can't kekulize after stripping hydrogens (for examplec1cc[cH-]c1). Before, one such molecule aborted the whole library.GetBestRMSgets gpuma's automorphism cap (2e6) asmaxMatches. RDKit's default of 1e6 cut off molecules that were still within budget.Config and smaller fixes
batch_optimization_modeandlogging_levelmust be known values.1.5was silently truncated.steps_between_swapsmust be at least 1.model_cache_diris Fairchem-only: setting it for ORB or SevenNet is an error, and a directory that can't be created is an error. Both are re-checked after--model-type.cuda:Ncurrent again.model.pathnames a loaded checkpoint. The log summary and batch comments use it, and the summary counts unconverged structures..gpuma-versionsaysunknowninstead ofNonewhen gpuma has no package metadata.--jsononly gains keys (structures[].converged,n_generated,model.path), soSUMMARY_SCHEMA_VERSIONstays 1.Docs
docs/config.md,docs/cli.md, the agent skill and the example configs (max_steps) are updated.Testing
On catalyst (4× Quadro RTX 6000, driver 535):
mkdocs build --strictpasses, and the three Python examples run cleanly.torch._inductoralready being imported by an earlier test.